-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
modernize code using java 17 syntax #10889
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Thanks for your pull request! This pull request does not follow the contribution rules. Could you have a look? ❌ All commit messages should start with a JIRA issue key matching pattern › This message was automatically generated. |
hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/CUBRIDDialect.java
Outdated
Show resolved
Hide resolved
...rnate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2LegacyDialect.java
Outdated
Show resolved
Hide resolved
...ects/src/main/java/org/hibernate/community/dialect/GaussDBCastingIntervalSecondJdbcType.java
Outdated
Show resolved
Hide resolved
...-dialects/src/main/java/org/hibernate/community/dialect/pagination/AltibaseLimitHandler.java
Outdated
Show resolved
Hide resolved
...ialects/src/main/java/org/hibernate/community/dialect/pagination/LegacyHSQLLimitHandler.java
Outdated
Show resolved
Hide resolved
...nity-dialects/src/main/java/org/hibernate/community/dialect/pagination/RowsLimitHandler.java
Outdated
Show resolved
Hide resolved
...ects/src/main/java/org/hibernate/community/dialect/pagination/SQLServer2005LimitHandler.java
Outdated
Show resolved
Hide resolved
...ects/src/main/java/org/hibernate/community/dialect/pagination/SQLServer2005LimitHandler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks.
Many of these changes I like, but:
- I'm not at all a fan of getting rid of
else
everywhere. To me this just disrupts the symmetry of the code. I meanfalse
andtrue
are symmetric, why shouldtrue
be more nested? I do, on the other hand, like the?:
operator, which feels more "functional" to me. - I have not generally been replacing everything with
var
. I'm usually just doing it for longer type names and especially generic types. I definitely don't want to go through the whole code base and replace primitives,String
,Integer
, andObject
withvar
everywhere. That just doesn't buy us anything, really. So for consistency, I would prefer that we avoidvar
in those "trivial" cases. - I have not been writing
instanceof final
because it has too much tendency to make longif
conditions even longer. For consistency across the code base, let's stick with justinstanceof
.
55140f5
to
e968b15
Compare
3b31d77
to
2b44e52
Compare
2b44e52
to
fc485f3
Compare
Dear @gavinking , Thank you for the attention you gave to this PR and sorry for the extra mental load it might have caused.
Thanks @gavinking 🙏 |
e34e872
to
91b429f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Thank you @MoadElfatihi |
Hello,
This PR is about some changes to use jdk 17 syntaxe (no behavioral change).
Just a contribution to give a hand with ongoing codebase cleanup work 👍
Thanks
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.